Skip to content

πŸ› fix: silence the credential helper's audit-log append and pre-create the log so it records - #6106

Open
gregoryhunt wants to merge 1 commit into
hivecommons:v4from
gregoryhunt:fix/credential-helper-audit-log
Open

πŸ› fix: silence the credential helper's audit-log append and pre-create the log so it records#6106
gregoryhunt wants to merge 1 commit into
hivecommons:v4from
gregoryhunt:fix/credential-helper-audit-log

Conversation

@gregoryhunt

Copy link
Copy Markdown
Contributor

Summary

  • Every agent clone, fetch, and push printed git-credential-hive.sh: line 157: /var/run/hive-metrics/token-access.jsonl: Permission denied into the agent's pane. The helper appends an audit line with >> "$LOG" 2>/dev/null || true, but 2>/dev/null only mutes the printf: when the redirection itself fails, the shell reports it on its own stderr before the command runs. gh-wrapper.sh had this exact bug and fixed it with a group wrap in gh wrapper: pr/issue edit fails closed when the injected provenance label is missing; ensure-labels cache is repo-blindΒ #4043; the credential helper never got the same treatment.
  • The redirection always fails on a per-UID hive: /var/run/hive-metrics is 0755 dev:node by design (gh wrapper: author-gated list is fail-closed for ALL staff agents β€” gh api user is 403 for App installation tokens (#3982 regression)Β #4044, protecting the bot-identity file under agent-tokens/) and nothing ever created token-access.jsonl, so no agent UID can open it. Consequence: the audit log that both writers feed, and that GET /api/token-access reads, has been empty on every per-UID hive.
  • bin/git-credential-hive.sh: wrap the append in { ...; } 2>/dev/null || true, matching gh-wrapper.sh.
  • src/deploy/entrypoint.sh: pre-create the log as dev:node mode 0664 next to the existing hive-metrics setup so appends from both writers land. The directory stays 0755; only this one file becomes group-writable.

Observed on a live standalone hive (2026-09-05): the line appeared on every gh repo clone and git push from the quality agent, and /var/run/hive-metrics/token-access.jsonl did not exist while /api/token-access reported no audit log.

Related issues

Follows the pattern from #4043; respects the directory mode rationale in #4044.

Testing

  • bash bin/test_git_credential_hive.sh β€” 16 passed. The new assertion runs a get and fails if token-access.jsonl, Permission denied, or No such file appears in the combined output. The harness has no /var/run/hive-metrics, so it reproduces the failing redirection exactly: red on the old helper (15 passed, 1 failed), green on the new one.
  • bash -n on both shell files.
  • cd src && go build ./... / go test ./... β€” not run; no Go changes.
  • Other / not run (explain): entrypoint change verified by reading only; it mirrors the adjacent mkdir/chown/chmod lines and is || true guarded like them.

Contributor checklist

  • PR targets v4 per CONTRIBUTING.md.
  • Title uses the repo emoji convention.
  • Commits include DCO sign-off (git commit -s).
  • Docs, examples, and policies are updated when behavior changes (none affected).
  • changelog.d/fixed-credential-helper-audit-log.md carries the changelog entry.
  • No secrets, credentials, or local runtime state are committed.

πŸ€– Generated with Claude Code

https://claude.ai/code/session_01CoYvUkv8Gp4AHGWLo97nqb

…e the log so it records

Every agent clone, fetch, and push printed

  git-credential-hive.sh: line 157: /var/run/hive-metrics/token-access.jsonl: Permission denied

into the agent's pane. The helper appends an audit line with
`>> "$LOG" 2>/dev/null || true`, but `2>/dev/null` only mutes the printf:
when the redirection itself fails the shell reports it on its own stderr
before the command runs. gh-wrapper.sh had this exact bug and fixed it by
wrapping the append in a group (hivecommons#4043); the credential helper never got
the same treatment.

The redirection always failed on a per-UID hive: /var/run/hive-metrics is
0755 dev:node by design (hivecommons#4044, protecting the bot-identity file under
agent-tokens/) and nothing ever created token-access.jsonl, so no agent
UID could open it. That also means the audit log both writers feed β€”
and GET /api/token-access reads β€” has been empty on every per-UID hive.

- bin/git-credential-hive.sh: wrap the append in `{ ...; } 2>/dev/null`
  so a failed redirection is silent, matching gh-wrapper.sh.
- src/deploy/entrypoint.sh: pre-create the log as dev:node 0664 next to
  the existing hive-metrics setup so appends from both writers land. The
  directory stays 0755; only this one file is group-writable.
- bin/test_git_credential_hive.sh: assert a `get` never emits
  token-access.jsonl / Permission denied noise. The harness has no
  /var/run/hive-metrics either, so it reproduces the leak exactly; the
  assertion fails on the old helper and passes on the new one.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CoYvUkv8Gp4AHGWLo97nqb
Signed-off-by: Gregory Hunt <greg@on-board.ai>
@kubestellar-prow kubestellar-prow Bot added the dco-signoff: yes Indicates the PR's author has signed the DCO. label Sep 5, 2026
@kubestellar-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign hanthor for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubestellar-prow kubestellar-prow Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 5, 2026
@kubestellar-hive

Copy link
Copy Markdown
Contributor

Scanner review β€” one finding (Medium)

The { ...; } 2>/dev/null || true wrap and the leak-regression test check out. But the pre-create step changes the audit log's trust model:

The token-access audit log becomes truncatable/forgeable by every audited agent UID (src/deploy/entrypoint.sh, the new pre-create block)

The PR pre-creates /var/run/hive-metrics/token-access.jsonl as dev:node mode 0664, and every agent UID is in group node. Group write is not append-only: any agent can reopen the file O_TRUNC/O_WRONLY and wipe or rewrite the entire trail β€” including entries recording other agents' token accesses. This file is explicitly a security artifact: src/pkg/dashboard/api.go (~line 2171) marks it "SECURITY (#3936, CWE-284): the token-access log records every gh CLI token access", and GET /api/token-access is owner-gated precisely so agents can't read it β€” yet after this change any agent can silently erase it. Neither the entrypoint nor the readers detect truncation. The adjacent agent-tokens hardening comment (entrypoint.sh ~705–708) shows the directory was kept non-agent-writable specifically to prevent this class of tampering.

Suggested: have the trusted side own the writes (agents emit to a dev-owned FIFO/collector), or at minimum chattr +a (append-only) where the filesystem supports it, and document that the log is agent-tamperable otherwise.


Advisory review by scanner agent (ACMM L5 β€” hold-gated mode). Not a merge gate.

β€” hive: agent=scanner backend=copilot model=claude-fable-5

@kubestellar-hive

Copy link
Copy Markdown
Contributor

Scanner review β€” verified, one security tradeoff worth stating on the record.

Verified locally (branch pr-6106 at a live per-UID host): the redirection-leak analysis is correct β€” >> f 2>/dev/null applies the redirections left-to-right, so a failed >> open reports on the shell's original stderr before 2>/dev/null ever takes effect; wrapping in { ...; } 2>/dev/null is the right shape and matches gh-wrapper.sh (#4043). The new harness assertion reproduces the leak exactly and passes on this branch (the 2 unrelated mode-notice failures on this host are pre-existing environment sensitivity, now filed as #6178 with a fix in #6179).

Tradeoff to acknowledge: chmod 664 + group node on token-access.jsonl makes the token-access audit log writable by every agent UID β€” an agent can truncate or rewrite its own access trail (> /var/run/hive-metrics/token-access.jsonl succeeds). That's inherent to letting agent-UID processes append without a writer daemon, and the entrypoint comment is right that the bot-identity file stays untouched β€” but the doc/endpoint consumers should treat this log as operational visibility, not tamper-evident audit. Worth a one-line note where GET /api/token-access is documented.

Scanner agent (ACMM L5 β€” hold-gated mode); advisory review, no merge action.

β€” hive: agent=scanner backend=copilot model=claude-fable-5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco-signoff: yes Indicates the PR's author has signed the DCO. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant